Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Fix failures on typescript@next #2789

Merged
merged 2 commits into from
May 20, 2017
Merged

Fix failures on typescript@next #2789

merged 2 commits into from
May 20, 2017

Conversation

andy-hanson
Copy link
Contributor

@andy-hanson andy-hanson commented May 19, 2017

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Overview of change:

TypeScript has made some breaking changes recently (microsoft/TypeScript#15486, microsoft/TypeScript#15594, microsoft/TypeScript#15887), so tslint must be updated.

Waiting on ajafff/tsutils#3.

Copy link
Contributor

@ajafff ajafff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM with comments

@@ -389,7 +389,8 @@ class CompletedDocsWalker extends Lint.ProgramAwareRuleWalker {
}

private checkNode(node: ts.Declaration, nodeType: DocType): void {
if (node.name === undefined) {
const { name } = node as ts.NamedDeclaration;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just update the type annotation of the parameter

package.json Outdated
@@ -72,7 +72,7 @@
"rimraf": "^2.5.4",
"tslint": "latest",
"tslint-test-config-non-relative": "file:test/external/tslint-test-config-non-relative",
"typescript": "^2.3.0"
"typescript": "next"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if it makes sense to use typescript@next in development while having a yarn.lock. I guess there'll be many merge conflicts just because of the typescript version in yarn.lock

Copy link
Contributor

@adidahiya adidahiya May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I don't want this change in development

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't compile in typescript pre-next though. Could I just tag it to the specific daily version?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would that mean that tslint API consumers would need to upgrade to 2.4.0-dev as well? I'm very wary of making this change; even if the compiled tslint typings continue to work with 2.3 in this PR, bumping to a nightly version opens up the door to future API breaks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nchen63 has another approach to fixing master that compiles with ts 2.3: https://github.com/palantir/tslint/tree/fix-master

@ajafff
Copy link
Contributor

ajafff commented May 19, 2017

@andy-hanson I released tsutils@2.0.0 with your change. That should fix the build

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants